Skip to content

Support building integration packages with required input dependencies#3459

Open
teresaromero wants to merge 32 commits intoelastic:mainfrom
teresaromero:feat/3277-composable-input-bundling
Open

Support building integration packages with required input dependencies#3459
teresaromero wants to merge 32 commits intoelastic:mainfrom
teresaromero:feat/3277-composable-input-bundling

Conversation

@teresaromero
Copy link
Copy Markdown
Contributor

@teresaromero teresaromero commented Apr 14, 2026

Summary

Composable (integration) packages can declare requires.input so that, at build time, elastic-package downloads those input packages from the Elastic Package Registry (or the configured registry URL), then updates the built package: agent templates are bundled with Fleet-correct template_paths order, variables are merged into the integration and data stream manifests, field definitions are merged into data stream fields/, and package: references on policy inputs and data stream streams are replaced with the concrete input type from the resolved input package. This aligns with the composable packages direction in package-spec#1083.

What ships (sub-issues / phases)

  1. Agent template bundling#3278: download required input packages, copy policy and data stream templates, order template_paths (input templates first, integration last).
  2. Variable merge#3279: merge variable definitions from input packages into the composable manifest and data stream manifests (promotion, overrides, duplicates).
  3. Field bundling#3280: copy field definitions from input packages into built data streams; integration-defined fields win.
  4. Stream / input resolution#3380: replace package: with resolved type: / input: so the built output matches hand-authored packages.

Implementation

  • internal/requiredinputs/RequiredInputsResolver implements Bundle(buildPackageRoot) (nil resolver is a no-op). Pipeline: templates → variables → fields → stream resolution.
  • internal/registry/DownloadPackage, TLS options, tests.
  • internal/packages/ — manifest types for requires.input.
  • internal/builder/packages.go — wires RequiredInputsResolver into the build.
  • Commands — registry base URL resolution aligns with install: build prefers active profile (stack.epr.base_url) when available and falls back to global config (package_registry.base_url) and then production.

Package signature verification (deferred)

Detached signature verification for downloaded packages was explored during review, but is deferred to a follow-up to finalize the key distribution/rotation approach and defaults.

Follow-ups (out of scope for this PR)

Tracked issues:

Other follow-ups mentioned during review (not yet tracked in elastic-package):

  • Local cache for downloaded required input packages.
  • Converge local registry serving methods.
  • Refactor methods with PackageRoot as struct members.

How to test

go test ./...
make build format lint licenser gomod update

Support requires.input (and related manifest fields) for composable
integration packages per package-spec.

Made-with: Cursor
Download integration zip artifacts from EPR for required input
resolution during build.

Made-with: Cursor
Verify detached signatures when enabled via environment configuration.

Made-with: Cursor
Download required input packages, copy policy and data stream agent
templates (Fleet merge order), merge manifest variables, bundle data
stream field definitions, and resolve package: stream references to
concrete input types.

Made-with: Cursor
Use profile-aware registry URLs for install, test, benchmark, and
script runners; inject RequiredInputsResolver into the build pipeline.

Made-with: Cursor
Fixtures cover template bundling, variable merge scenarios, field
bundling, linked template_path, and stream resolution.

Made-with: Cursor
Document requires.input build behavior and how to use a local or custom
EPR during development.

Made-with: Cursor
Handle closeFn errors in defer, trim always-nil error returns from merge
helpers, add gocognit nolint for mergeVariables, and fix test assignments.

Made-with: Cursor
teresaromero and others added 3 commits April 14, 2026 10:19
Package archetypes have no required input dependencies, so
BuildOptions.RequiredInputsResolver can be left nil. The builder already
falls back to NoopRequiredInputsResolver when the field is unset, making
the mock redundant.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract per-manifest logic from bundleDataStreamTemplates into a new
unexported processDataStreamManifest method, and add unit tests covering
read failure, invalid YAML, unknown package (no write-back guard), partial
stream errors, and no-package stream skipping.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Files helper

Extract the duplicated copy loop from collectAndCopyInputPkgPolicyTemplates and
collectAndCopyInputPkgDataStreams into a single collectAndCopyPolicyTemplateFiles
function parameterised by destDir. Remove the RequiredInputsResolver receiver from
both wrappers since they don't use resolver state. Add unit tests for the new helper
covering single/multiple template paths, deduplication, missing files, invalid paths,
custom destDir, and content preservation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@teresaromero
Copy link
Copy Markdown
Contributor Author

teresaromero commented Apr 14, 2026

Review Comments Addressed

This PR consolidates the work from #3329 and addresses all review comments. Summary below.


@jsoriano

File naming — no underscores in Go filenames
The builder code was fully refactored into a new internal/requiredinputs/ package with clean filenames (requiredinputs.go, copy.go, fields.go, policytemplates.go, streams.go, streamdefs.go, variables.go, yamlutil.go).

Rename processDSManifest
Renamed to processDataStreamManifest in internal/requiredinputs/streams.go.

Rename resolveInputPackage
Renamed to mapRequiredInputPackagesPaths in internal/requiredinputs/requiredinputs.go, which better reflects that it downloads and maps multiple packages.

Reuse core logic from ReadAllPackageManifests
Introduced openPackageFS() as a shared helper (handles both directory and zip packages) and reused packages.ReadDataStreamManifestBytes / packages.ReadPackageManifestBytes across copy.go, fields.go, and streams.go.

Variable naming matching YAML key
PackageDependency.Package now uses yaml:"package" tag in internal/packages/packages.go.

YAML format preservation vs. struct approach
Tried the struct approach — marshalling caused data loss (comments, ordering, unknown fields). Keeping the yaml.Node tree manipulation to preserve the manifest faithfully. The built package manifest is modified in-place with targeted key upserts/removals.

Use slices package instead of custom remove helper
removeKey in yamlutil.go uses slices.IndexFunc + slices.Delete(idx, idx+2). Using slices.DeleteFunc directly is not applicable here because YAML mapping nodes store key-value pairs as two consecutive elements in Content; you need to delete both atomically, which requires the index-based Delete(idx, idx+2) form.

Zip reader as filesystem instead of extracting
DownloadPackage in internal/registry/client.go now saves the zip file as-is (no extraction). openPackageFS() in requiredinputs.go opens it with zip.OpenReader + fs.Sub, presenting it as an fs.FS to all consumers.

Content packages in dependency docs
docs/howto/dependency_management.md now covers both input packages (resolved at build time) and content packages (resolved at runtime by Fleet).

Higher-level abstraction in factory.go
installer.Options.RequiredInputsResolver is typed as the requiredinputs.Resolver interface. NoopRequiredInputsResolver is used where no bundling is needed. The TODO for local-path override is noted in the struct comment.

Local cache for packages — deferred to a follow-up as agreed.

Converge local registry serving methods — noted; left as a longer-term architectural refactor.

Methods with PackageRoot as struct members — noted; left for a future refactor.


@mrodm

EPR URL should account for profile settings
stack.PackageRegistryBaseURL(profile, appConfig) is now a public function in internal/stack/registry.go and is called from cmd/install.go. The build command does not use a profile, so it correctly reads from application config only.

Docker image reference in docs
Updated to docker.elastic.co/package-registry/package-registry:v1.37.0 in docs/howto/local_package_registry.md.

Port conflict warning
local_package_registry.md now explicitly warns to use a port other than 8080 to avoid conflicting with elastic-package stack up.

os.RootFS vs os.DirFS
os.OpenRoot() is used in requiredinputs.go (both for the build root and for directory-based input packages). root.FS() returns a sandboxed fs.FS that blocks symlink traversal outside the root directory.

Linked files / symlinks in policy templates
All file reads go through fs.ReadFile(inputPkgFS, ...) on the FS returned by openPackageFS. For directory packages os.OpenRoot prevents escaping the package directory. For downloaded packages the FS is derived from zip.OpenReader, which has no real symlinks.

Package signature validation
PGP signature verification is implemented in internal/registry/client.go. When ELASTIC_PACKAGE_VERIFY_PACKAGE_SIGNATURE=true, the client downloads the detached .sig file and verifies it via files.VerifyDetachedPGP() before returning the package path. The verifier public key is configurable via ELASTIC_PACKAGE_VERIFIER_PUBLIC_KEYFILE.


Known follow-ups (not in scope for this PR)

These were flagged by @jsoriano during testing and agreed to be addressed separately:

@teresaromero teresaromero requested review from jsoriano and mrodm April 14, 2026 09:32
- Add test/packages/composable/01_ci_input_pkg and 02_ci_composable_integration
  for requires.input coverage (vars, fields, templates, mixed streams)
- Phase-2 build in test-build-install-zip.sh after stack up; set
  package_registry.base_url to local EPR with restore in cleanup
- Mirror composable skip + phase-2 in test-build-install-zip-file.sh
- Point internal/requiredinputs integration tests at composable fixtures
- Remove duplicate manual_packages; edge fixtures use ci_input_pkg;
  refresh manual_packages README

Made-with: Cursor
Complete policy and data stream variable metadata (type, titles, flags)
for package-spec validation. Use logfile for the native logs stream so
Kibana accepts the data stream manifest. Update requiredinputs tests and
the manual required_inputs fixture to match.

Made-with: Cursor
test-build-zip runs after test-build-install-zip without a local registry;
building 02_ci_composable_integration would download ci_input_pkg from
production EPR and fail with 404. Mirror the install-zip phase-1 skip.

Made-with: Cursor
Comment thread internal/files/verify.go Outdated
Comment thread internal/registry/client.go Outdated
Comment thread internal/registry/client.go Outdated
Comment thread internal/requiredinputs/fields_test.go Outdated
Comment thread internal/requiredinputs/fields_test.go Outdated
Comment thread scripts/test-build-install-zip-file.sh Outdated
Comment thread scripts/test-build-install-zip-file.sh Outdated
Copy link
Copy Markdown
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with the package in #3432 and it seems to work.

As the change is pretty big, and it seems to work, maybe we can fix the most urgent things, merge it, and leave the rest for smaller follow ups.

Comment thread internal/registry/client.go Outdated
Comment thread internal/requiredinputs/streamdefs.go Outdated
Comment thread internal/requiredinputs/yamlutil.go Outdated

// mappingValue returns the value node for the given key in a YAML mapping node,
// or nil if the key is not present.
func mappingValue(node *yaml.Node, key string) *yaml.Node {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have internal/yamledit, that has methods for getting, setting, deleting and printing yaml nodes. They could likely be reused here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you have migrated the code to goccy/go-yaml. Did you consider the higher-level yamledit? We can also refactor this in a follow up, or look at how we can reuse helpers among the different cases of yaml edition.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I migrated to align with the library used but the methods where not common, so still there are "custom" methods used on the requiredinputs package. I think we could use some refactor for handling the yaml edits. I will open an issue for that too.

Comment thread internal/requiredinputs/variables.go Outdated
Comment thread internal/requiredinputs/variables.go Outdated
Comment thread internal/requiredinputs/requiredinputs.go
Comment thread internal/registry/client.go Outdated
// the top level) and a close function that must be called when done. For
// directory packages it closes the os.Root; for zip packages it closes the
// underlying zip.ReadCloser.
func openPackageFS(pkgPath string) (fs.FS, func() error, error) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might support only packages in zip format.

By now we are only supporting packages from the registry, that will come in this format. If we support local packages we can build them too.

Comment thread internal/requiredinputs/streamdefs.go
Comment thread internal/requiredinputs/streamdefs.go
NewClient now returns (*Client, error) so TLS/CA setup failures from
newHTTPClient are visible to callers. DownloadPackage defers zip removal
on failure after write, extracts verifyPackage for detached PGP checks,
and defers closing the zip file during verification.

Update all registry.NewClient call sites. Add revisionsFromRegistry in
script tests to keep Run under gocyclo limits.

Add tests for invalid CA paths/PEM, non-OK download responses, write
failures, TLSSkipVerify construction, and revisionsFromRegistry behavior.

Made-with: Cursor
Use stack.PackageRegistryBaseURL after loading the profile so
elastic.epr.url matches install, test, and benchmark.

Made-with: Cursor
Extract mergeVariables and resolveStreamInputTypes into focused helpers,
rename promoted var scope types, and warn when input packages define multiple
policy templates. Use errors.Is(os.ErrNotExist) in field bundling tests.

Add unit tests for promoted override scoping and stream input resolution with
multi-template input packages. Document new helpers and updated entry points.

Made-with: Cursor
Buildkite runs test-build-install-zip-file.sh which uses yq to override
package_registry.base_url for the composable phase-2 build. Include
test-build-install-zip-file and test-build-install-zip-file-shellinit in
the same with_yq branch as test-build-install-zip.

Made-with: Cursor
Run elastic-package stack shellinit only when -s is used; apply manual
exports only otherwise so the non-shellinit path is not polluted by
shellinit exports.

Made-with: Cursor
- Introduced new targets for composable integration tests in the Makefile.
- Updated the Buildkite pipeline to include jobs for the new composable test targets.
- Enhanced the integration test script to handle composable-only builds.
- Modified the test-build-install-zip-file.sh script to support composable input packages.

These changes facilitate dedicated testing for composable packages, improving CI coverage and ensuring proper integration.
Removed Windows-specific skip condition and adjusted directory handling in TestDownloadPackage_writeFailureCleansUp. The test now uses a temporary directory for zip file creation, ensuring proper cleanup after write failures. This enhances test reliability across platforms.
Comment thread cmd/build.go
Replace all gopkg.in/yaml.v3 usage in internal/requiredinputs with
github.com/goccy/go-yaml, aligning the package with internal/yamledit.
Rewrite yamlutil.go to operate on goccy/go-yaml AST types and reuse
yamledit.NewDocumentBytes for parsing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Deleted the package signature verification code from the project, including related tests and environment variable documentation. This simplifies the codebase and removes unused functionality related to verifying downloaded packages from the Package Registry.
@mrodm
Copy link
Copy Markdown
Contributor

mrodm commented Apr 15, 2026

/test

…tails

Clarified the configuration settings for package dependencies in the `dependency_management.md` and `local_package_registry.md` files. Updated references to `stack.epr.base_url` for profile-specific settings and improved the explanation of registry URL resolution priorities for various `elastic-package` commands.
@teresaromero teresaromero marked this pull request as ready for review April 16, 2026 07:55
Force: opts.installedPackage, // Force re-installation, in case there are code changes in the same package version.
RepositoryRoot: r.repositoryRoot,
SchemaURLs: r.schemaURLs,
RequiredInputsResolver: &requiredinputs.NoopRequiredInputsResolver{},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To run the system tests of a composable package, this package should be installed via this code as a requisite. I guess that if needed the package should be bundled before being installed, is that right ? Is it intended to be done this in a follow-up PR ?

I see that for policy tests there is a parameter in the runner r.requiredInputsResolver.

I think it would be needed to do something similar as in policy tests here to be able to build the composable packages in system tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i wont be addressing the testing of composable packages on this pr. just the bundling (build); following this comment #3459 (comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! Agreed, it makes sense to address that in a follow-up 👍

Force: installedPackage, // Force re-installation, in case there are code changes in the same package version.
RepositoryRoot: r.repositoryRoot,
SchemaURLs: r.schemaURLs,
RequiredInputsResolver: &requiredinputs.NoopRequiredInputsResolver{},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned for system tests, as this tests require to install the package it looks to me that it would be needed to be able to use a r.requiredInputsResolver parameter as in policy tests. WDYT ? Maybe it is expected to be done in a follow-up ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've narrowed the implementation of the bundling on build of the composable package. The noop resolver is here as a placeholder but it should be resololved in the followup, probably here #3464 ; when we also enable the overriding of the depencency

Absent: true,
Force: true,
RepositoryRoot: root, // Apparently not required, but adding for safety.
RequiredInputsResolver: &requiredinputs.NoopRequiredInputsResolver{},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Absent is set to true here, I think it could be set here the Noop struct

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

Force: true,
RepositoryRoot: root,
SchemaURLs: fields.NewSchemaURLs(fields.WithECSBaseURL(ecsBaseSchemaURL)),
RequiredInputsResolver: &requiredinputs.NoopRequiredInputsResolver{},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it require here to set an inputs resolver created beforehand as in policy tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as mentioned on the comment above, this is a placeholder to address when testing is enabled for composable

Copy link
Copy Markdown
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! It looks like most issues have been addressed or have follow up issues. I'd say we can merge this, and iterate on smaller PRs.

Comment thread internal/requiredinputs/yamlutil.go Outdated

// mappingValue returns the value node for the given key in a YAML mapping node,
// or nil if the key is not present.
func mappingValue(node *yaml.Node, key string) *yaml.Node {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you have migrated the code to goccy/go-yaml. Did you consider the higher-level yamledit? We can also refactor this in a follow up, or look at how we can reuse helpers among the different cases of yaml edition.

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Apr 16, 2026

💔 Build Failed

Failed CI Steps

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants